Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Calculating Risk in VulnerableCode #1593

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

ziadhany
Copy link
Collaborator

@ziadhany ziadhany commented Sep 17, 2024

issue: #1543

image

image

Screenshot from 2024-10-28 18-33-27

Screenshot from 2024-10-29 15-48-52

@TG1999 TG1999 added the 2-next label Oct 15, 2024
@DennisClark
Copy link
Member

it would be really great to see a review of this PR soon!

Set data_source as the header for the exploit table.
Squash the migration files into a single file.
Add test for exploit-db , metasploit
Add a missing migration file
Rename resources_and_notes to notes
Fix Api test
Refactor metasploit , exploitdb , kev improver
Rename Kev tab to exploit tab
Add support for exploitdb , metasploit, kev

Signed-off-by: ziadhany <[email protected]>
Signed-off-by: ziad hany <[email protected]>
Refactor the error handling logic in the code.

Signed-off-by: ziadhany <[email protected]>
Signed-off-by: ziad hany <[email protected]>
Add pipeline_id for ( kev, metasploit, exploit-db )

Signed-off-by: ziadhany <[email protected]>
Signed-off-by: ziad hany <[email protected]>
Set data_source as the header for the exploit table.
Squash the migration files into a single file.
Add test for exploit-db , metasploit
Add a missing migration file
Rename resources_and_notes to notes
Fix Api test
Refactor metasploit , exploitdb , kev improver
Rename Kev tab to exploit tab
Add support for exploitdb , metasploit, kev

Signed-off-by: ziadhany <[email protected]>
Signed-off-by: ziad hany <[email protected]>
Refactor the error handling logic in the code.

Signed-off-by: ziadhany <[email protected]>
Signed-off-by: ziad hany <[email protected]>
Signed-off-by: ziadhany <[email protected]>
Signed-off-by: ziad hany <[email protected]>
Signed-off-by: ziadhany <[email protected]>
Signed-off-by: ziad hany <[email protected]>
Signed-off-by: ziadhany <[email protected]>
Signed-off-by: ziad hany <[email protected]>
Signed-off-by: ziadhany <[email protected]>
Signed-off-by: ziad hany <[email protected]>
uncomment all importers

Signed-off-by: ziad hany <[email protected]>
@ziadhany ziadhany requested a review from TG1999 October 22, 2024 13:40
@TG1999
Copy link
Contributor

TG1999 commented Oct 28, 2024

@ziadhany this generally looks good to me, can we have some logs for the pipeline and some UI and API screenshots so we can merge it.

Copy link
Member

@keshav-space keshav-space left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ziadhany, some nits for your consideration.
Also, please use LoopProgress while iterating over large records, as we do here

progress = LoopProgress(total_iterations=fetched_exploit_count, logger=self.log)

vulnerabilities/pipelines/risk_package.py Outdated Show resolved Hide resolved
vulnerabilities/pipelines/risk_package.py Outdated Show resolved Hide resolved
vulnerabilities/risk.py Outdated Show resolved Hide resolved
vulnerabilities/pipelines/risk_package.py Outdated Show resolved Hide resolved
vulnerabilities/pipelines/risk_package.py Outdated Show resolved Hide resolved
vulnerabilities/pipelines/risk_package.py Outdated Show resolved Hide resolved
vulnerabilities/risk.py Outdated Show resolved Hide resolved
vulnerabilities/models.py Outdated Show resolved Hide resolved
vulnerabilities/migrations/0074_package_risk.py Outdated Show resolved Hide resolved
vulnerabilities/risk.py Outdated Show resolved Hide resolved
…skPackagePipeline to ComputePackageRiskPipeline. Add a tooltip for risk, and remove any unused imports in the view.

Signed-off-by: ziad hany <[email protected]>
…ore and remove any extra whitespace in views.py.

Signed-off-by: ziad hany <[email protected]>
@ziadhany
Copy link
Collaborator Author

@TG1999 @keshav-space
Thank you for reviewing this PR. Here are the pipeline logs:

/home/ziad-hany/PycharmProjects/vulnerablecode/venv/bin/python /home/ziad-hany/PycharmProjects/vulnerablecode/manage.py improve localhost:8000 --all 
Improving data using compute_package_risk
INFO 2024-10-28 13:10:07.074 Pipeline [ComputePackageRiskPipeline] starting
INFO 2024-10-28 13:10:07.074 Step [add_risk_package] starting
INFO 2024-10-28 13:10:07.755 Calculating risk for 84,396 affected package records
INFO 2024-10-28 13:14:57.360 Progress: 10% (8440/84396) ETA: 2601 seconds (43.4 minutes)
INFO 2024-10-28 13:17:30.519 Progress: 20% (16880/84396) ETA: 1768 seconds (29.5 minutes)
INFO 2024-10-28 13:19:28.477 Progress: 30% (25319/84396) ETA: 1307 seconds (21.8 minutes)
INFO 2024-10-28 13:22:06.047 Progress: 40% (33759/84396) ETA: 1076 seconds (17.9 minutes)
INFO 2024-10-28 13:25:15.062 Progress: 50% (42198/84396) ETA: 907 seconds (15.1 minutes)
INFO 2024-10-28 13:30:57.706 Progress: 60% (50638/84396) ETA: 833 seconds (13.9 minutes)
INFO 2024-10-28 13:36:04.109 Progress: 70% (59078/84396) ETA: 667 seconds (11.1 minutes)
INFO 2024-10-28 13:39:06.350 Progress: 80% (67517/84396) ETA: 434 seconds (7.2 minutes)
INFO 2024-10-28 13:42:57.214 Progress: 90% (75957/84396) ETA: 219 seconds (3.6 minutes)
INFO 2024-10-28 13:48:50.555 Progress: 100% (84396/84396)
INFO 2024-10-28 13:48:50.694 Successfully added risk score for 84,396 package
INFO 2024-10-28 13:48:50.721 Step [add_risk_package] completed in 2324 seconds (38.7 minutes)
INFO 2024-10-28 13:48:50.721 Pipeline completed in 2324 seconds (38.7 minutes)

Process finished with exit code 0


@tdruez
Copy link
Contributor

tdruez commented Oct 29, 2024

@ziadhany @TG1999 Could we make a push on this one as this is a key addition for the VEX support in DejaCode.
Also, make sure it is available everywhere in the relevant API endpoint as we need to capture this data.

vulnerabilities/models.py Outdated Show resolved Hide resolved
vulnerabilities/pipelines/risk_package.py Outdated Show resolved Hide resolved
Copy link
Member

@keshav-space keshav-space left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ziadhany, see some suggestion.
Also, please rename the pipeline file from risk_package.py to compute_package_risk.py and the corresponding test file to test_compute_package_risk.py.

vulnerabilities/pipelines/risk_package.py Outdated Show resolved Hide resolved
vulnerabilities/pipelines/risk_package.py Outdated Show resolved Hide resolved
vulnerabilities/pipelines/risk_package.py Outdated Show resolved Hide resolved
Add pagination and refactor bulk_update_package

Signed-off-by: ziad hany <[email protected]>
@ziadhany
Copy link
Collaborator Author

@tdruez @keshav-space, I've completed the requested changes and I hope we can move forward and merge this PR.

Additionally, please review the weight_config.json file to ensure an accurate view of the risk. The default weight is set to one and may need adjustment to reflect typical risk values if we plan to use it in Dejacode.

Copy link
Member

@keshav-space keshav-space left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ziadhany, looking good! I ran the pipeline locally and it worked alright.
Before we merge this, let's discuss the weight configuration in our community call today.
Also, we will need to revisit the risk computation once #1597 is resolved.

@DennisClark
Copy link
Member

DennisClark commented Nov 5, 2024

note that @keshav-space is going to create a list of sources to expand the entries in weight-config.json and we'll take it from there. Please put suggested weights in the file. We probably also need a default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants